Skip to content

fix(webui): keep what the glyphs say, and give the slow tests a budget - #229

Merged
modacker merged 5 commits into
webuifrom
fix/webui-upstream-regression
Oct 22, 2026
Merged

modacker merged 5 commits into
webuifrom
fix/webui-upstream-regression

Conversation

@modacker

Copy link
Copy Markdown
Collaborator

What this fixes

Upstream 149979d0 ("perf(browser): reuse WebSocket for input and coalesce at 60 Hz") also replaced the hand-drawn WebUI icon set with a Tabler wrapper. That left webui red on every platform — run 39783884699 — and the specs that failed were the ones that noticed what the conversion changed by accident.

The red was in two gates, and the second one was hidden behind the first: test:webui failed before test:webui-browser ever ran.

1. Three glyphs stopped saying what they said

  • The pin. WebuiIconContextPin drew the outline pin for both states, so its pinned argument said nothing: a pinned session row looked exactly like an unpinned one. The filled pin says it now, the way the hand-drawn glyph's filled variant did. All three call sites — the pinned-session row, the expanded project row and the project pin — already pass the state, so no caller changes.
  • The submenu chevron. It was given an 18px box inside the 16px slot .webui-context-menu-chevron reserves for it, which squashes it. It draws at 16 again — the size the old glyph declared, and the size the other chevrons draw at.
  • The parent row's disclosure. The expandable parent rows were given the same outline chevron the project rows use, so the two kinds of expander looked the same again. The old icon set drew a filled arrow for them and said so in its own comment; the filled caret keeps that distinction, in Tabler's drawing.

2. Six browser specs were querying the old drawings

They read the pin's state off the clip path the hand-drawn glyph carried, and the Tabler glyph carries none, so the locator never resolved and the cases waited out their timeout. The state is now read off the glyph's own class; the rule about two pinned rows is the one that is left, which is that each row has an element of its own. The disclosure case pinned the path the hand-drawn triangle drew, and now asserts the filled caret it is drawn with — which is the claim its own name makes. The questionnaire's progress indicator is read as "1/3" between the two step affordances; the next one is an icon now, so it contributes no text to the indicator.

3. The transcript test had no browser to launch

packages/webui/test/integration/agent-transcript.test.ts is in the test:webui Vitest gate, which runs before the test:webui-browser gate installs Playwright's own Chromium. On CI the pinned browser was not on disk yet, so the launch failed with Executable doesn't exist at .../ms-playwright/chromium_headless_shell-1243/....

The runner's own Chrome is there, and Playwright drives it through the chrome channel, so the case now falls back to it. A launch that fails for any other reason is still the failure it looks like, and is not retried.

4. The dependency gate had no budget of its own

packages/webui/test/gates/webui-dependency-check.test.ts runs 99 cases that each build a TypeScript program, under Vitest's five-second default. On a loaded machine that budget turns into a failure that says nothing about the rule: the same case passes on an idle one, and a different case fails each time. The file now carries a 30-second budget of its own; the two cases that run the whole CLI keep their own 60 seconds.

How this was checked

  • pnpm test:webui — 207 passed, 5 skipped, 212 files; 3241 tests passed.
  • pnpm test:webui-browser — 155 passed, 1 failed: native-runtime.spec.mjs fails on posix_spawnp, which is this machine's node-pty and fails the same way on the untouched base.
  • The transcript case run with Playwright's browser directory pointed at an empty folder, which is the CI condition: it drives the runner's Chrome and passes.
  • The dependency gate run with its budget set to 100ms — 63 cases time out, so the budget is live — and at 30s, where all 117 pass.
  • typecheck:webui for the client and the test programs, and check:webui-dependency, all pass.

What this does not change

No expectation was relaxed to reach green. The pin states are still required to draw different marks, the parent row's disclosure is still required to be the filled one, the icon assertions still pin the drawn size, and the rule the dependency gate checks is the same one it checked before.

probe added 5 commits October 22, 2026 22:14
The tabler conversion drew the outline pin for both states, so the `pinned`
argument stopped saying anything: a pinned session row looked exactly like
an unpinned one. The filled pin says it now, the way the hand-drawn glyph's
filled variant did.

The same conversion gave the submenu chevron an 18px box inside the 16px
slot the context menu reserves for it, which squashes it. It draws at 16
again, the size the other chevrons draw at.

The specs that caught both now look for the glyph's own class rather than for
the clip path the hand-drawn glyph carried, and for the size that is drawn
rather than for a viewBox that belonged to the old drawing.
This file belongs to the `test:webui` Vitest gate, which runs before the
`test:webui-browser` gate installs playwright's own chromium, so on CI the
pinned browser is not on disk yet and the launch failed with "Executable
doesn't exist". The runner's own Chrome is there, and playwright drives it
through the `chrome` channel. A launch that fails for any other reason is
the failure it looks like, and is not retried.
Every case in this file builds a TypeScript program, so what a case costs is
the machine's speed rather than a fact about the rule it checks. Under
vitest's five-second default a loaded machine turned that into a failure
that said nothing about the gate: the same case passed on an idle machine,
and a different case failed each time. The budget is written down now, so
the gate reports the rule rather than the machine it ran on.
The tabler conversion gave the parent-session disclosure the same outline
chevron the project rows use, so the two kinds of expander looked the same
again. The filled arrow is kept for the expandable parent rows, which is
what the old glyph set drew and what the row's own comment said it was
retained for. It is a filled caret now, so the drawing is tabler's and the
distinction is still there.
The pin specs read the state off the clip path the hand-drawn glyph carried,
and the tabler glyph carries none, so the locator never resolved and the
cases waited out their timeout. The state is now read off the glyph's own
class, and the rule about two pinned rows is the one that is left: each row
has an element of its own.

The disclosure case pinned the path the hand-drawn triangle drew. It now
asserts the filled caret it is drawn with, which is the claim the case's
name makes.

The questionnaire's progress indicator is read as "1/3" between the two
step affordances. The next one is an icon now, so it contributes no text to
the indicator.
@modacker
modacker merged commit b8c21a2 into webui Oct 22, 2026
12 of 14 checks passed
@modacker
modacker deleted the fix/webui-upstream-regression branch October 23, 2026 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant